Code Review

Verdict: REQUEST CHANGES

Summary: This PR fixes two real bugs — a threading violation on @Published errorMessage in SystemAudioCapture and an overly broad path allowlist in FailedTranscriptionManager — and adds a solid regression test for the path fix. Both fixes are correct and well-implemented. However, the same threading bug that was fixed in SystemAudioCapture also exists in SCKAudioCapture and was missed.

Blockers (must fix before merge)

• SCKAudioCapture.swift:116 has the same threading bug this PR fixes in SystemAudioCapture. The start() method sets errorMessage = nil directly, but start() runs on a background thread (confirmed by the semaphore pattern at line 134 and the comment "synchronous wait since we're on a background thread"). Meanwhile line 151 already correctly dispatches to main for the error path. This should get the same publishErrorMessage treatment (or an inline DispatchQueue.main.async) for consistency and correctness.

Should fix (strongly recommended)

None.

Nits (take or leave)

None.

What looks good

• publishErrorMessage helper is clean: Thread.isMainThread avoids redundant dispatch when already on main, [weak self] prevents retain cycles. Good pattern.
• Path allowlist narrowing is a genuine security improvement. The old path captures/audio/ was broader than intended — the new captures/meetings/audio/ correctly scopes to only the transcript-owned archive subdirectory. The existing isFile(_:containedIn:) with resolvingSymlinksInPath() and trailing-slash normalization handles traversal and prefix-confusion attacks well.
• The regression test covers both the positive case (archived audio under transcripts/audio/ accepted) and the negative case (sibling directory outside transcript root rejected). Combined with the existing traversal, absolute-path, and out-of-home tests, the security boundary is well-covered.
• PR description is clear and verification steps are documented.

Generated by Claude Code